there were also plenty of Double_t...

@cortesep wait, I see you have same commits in this PR and in the earlier https://github.com/AliceO2Group/AliceO2/pull/5533/files. By amending both you will introduce conflicts.

I would suggest to do

git checkout dev_zdcawrw
git rebase working_branch

Add all fixes, make sure that it compiles and works both for mc->raw and raw->digis, the forcepush by
git push -f cortesep:working_branch

Then the old one can be closed.

@cortesep yes, if all the fixes (UShort_t etc) and comments are accounted in this one, just do push -f ...

You don't need : in the push, just git push -f cortesep working_branch

@cortesep ,
to rename the PedestalData to e.g. PedestalScalerData (or EndOfOrbitData, you decide), just do from O2 dir.:

git grep -l PedestalData | grep -i zdc | xargs perl -pi -e 's/PedestalData/PedestalScalerData/g'

Did you validate the changes on 2 PbPb data samples I put on cernbox? Is the content and the number of original and decoded digits now consistent?

To hide the hit information that is known only to simulation and is not available in raw data. I didn't know how to set this through
ZDCDigitizerSpec, get the flag in its init() method, like

AliceO2/Steer/DigitizerWorkflow/src/ITSMFTDigitizerSpec.cxx

Line 69 in 0ad5561

mDisableQED = ic.options().get<bool>("disable-qed");

and then set it to Digitizer. I would suggest to not store this info by defaults, i.e. the option should be something like

Options{{"enable-hit-info", o2::framework::VariantType::Bool, false, {"enable hit info of unread channels"}}

BTW, I would suggest also to propagate the useMC flag (--disable-mc of o2-sim-digitizer-workflow) which will allow to skip creation of labels info, see:

Once you finish, please remove [WIP]. You could not redigitize pbpb because I did not copy Hits and Kine, they are too boog. I'll redigitize both 1 and 4 TF versions and send to you for final validation.

To hide the hit information that is known only to simulation and is not available in raw data. I didn't know how to set this through
ZDCDigitizerSpec, get the flag in its init() method, like

AliceO2/Steer/DigitizerWorkflow/src/ITSMFTDigitizerSpec.cxx

Line 69 in 0ad5561

mDisableQED = ic.options().get<bool>("disable-qed");

and then set it to Digitizer. I would suggest to not store this info by defaults, i.e. the option should be something like

Options{{"enable-hit-info", o2::framework::VariantType::Bool, false, {"enable hit info of unread channels"}}

Done.

BTW, I would suggest also to propagate the useMC flag (--disable-mc of o2-sim-digitizer-workflow) which will allow to skip creation of labels info, see:

I have an issue with disable-mc options. If I introduce it in the list of options
return DataProcessorSpec{
"ZDCDigitizer",
Inputs{InputSpec{"collisioncontext", "SIM", "COLLISIONCONTEXT", static_cast(channel), Lifetime::Timeframe}},
outputs,
AlgorithmSpec{adaptFromTask()},
Options{{"enable-hit-info", o2::framework::VariantType::Bool, false, {"enable hit info of unread channels"}},
{"disable-mc", o2::framework::VariantType::Bool, false, {"do not store MC labels"}}}
};
I have an error
Error: option '--disable-mc' is ambiguous and matches different versions of '--disable-mc'

If the option is not in the list the program crashes..

I used a trick to set an option coherent with mctruth argument. I don't know if this is what you meant but it seems to work

I think we should not use an option with a different name here (disable-mc-zdc), but it should use the same disable-mc option of all other workflows, otherwise it is misleading and failure-prone.

I checked what the TPC workflow is doing, and it defines the disable-mc option:

void customize(std::vector<o2::framework::ConfigParamSpec>& workflowOptions)
{
  using namespace o2::framework;
  std::vector<ConfigParamSpec> options{
[...]
    {"disable-mc", VariantType::Bool, false, {"disable sending of MC information"}},
[...]
  };
  std::swap(workflowOptions, options);
}

I am wondering why it should be ambiguous here. Due to the different option description perhaps?

{"disable-mc-zdc", o2::framework::VariantType::Bool, false, {"do not store ZDC MC labels"}}}

But in any case, I think we should understand and fix that.

Only remaining difference is that Digitizer writes a single entry for all events, while raw to digits writes one entry per TF

@cortesep You don't need to add disable-mc on your digitizer device level, it is already defined on the workflow level:

So, you just need to add to getZDCDigitizerSpec and getZDCDigitWriterSpec an argument (bool usMC) and if it is on, skip everything related to MC labels, like others do.

Only remaining difference is that Digitizer writes a single entry for all events, while raw to digits writes one entry per TF
This is by design.

Didn't realize that I could change the constructor. Now it should be ok.

No, there are conflicts, could you rebase to fresh dev and resolve the conflicts?

Hi @shahor02 there are only rebase conflicts, you can squash merge already now.
Rebasing doesn't work since apparently there were merge commits in between, that solved some conflicts in between, so just rebasing the individual commits is not possible.

@davidrohr right, thanks!